Skip to content

Conversation

@s-perron
Copy link
Contributor

@s-perron s-perron commented May 6, 2025

When the DataLayout is destroyed, the memory backing Offsets is
released. This causes a use after free.

To fix it, I added a DataLayout varible that will not be destroyed until
after Offsets is used.

Fixes asan failure caused by #135789

When the DataLayout is destroyed, the memory backing `Offsets` is
released. This causes a use after free.

To fix it, I added a DataLayout varible that will not be destroyed until
after Offsets is used.

Fixes asan failure caused by llvm#135789
@llvmbot
Copy link
Member

llvmbot commented May 6, 2025

@llvm/pr-subscribers-backend-spir-v

Author: Steven Perron (s-perron)

Changes

When the DataLayout is destroyed, the memory backing Offsets is
released. This causes a use after free.

To fix it, I added a DataLayout varible that will not be destroyed until
after Offsets is used.

Fixes asan failure caused by #135789


Full diff: https://github.com/llvm/llvm-project/pull/138695.diff

1 Files Affected:

  • (modified) llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp (+2-1)
diff --git a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
index 35ddb906c366a..948869d5c5260 100644
--- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
@@ -2062,8 +2062,9 @@ void SPIRVGlobalRegistry::updateAssignType(CallInst *AssignCI, Value *Arg,
 
 void SPIRVGlobalRegistry::addStructOffsetDecorations(
     Register Reg, StructType *Ty, MachineIRBuilder &MIRBuilder) {
+  DataLayout DL;
   ArrayRef<TypeSize> Offsets =
-      DataLayout().getStructLayout(Ty)->getMemberOffsets();
+      DL.getStructLayout(Ty)->getMemberOffsets();
   for (uint32_t I = 0; I < Ty->getNumElements(); ++I) {
     buildOpMemberDecorate(Reg, MIRBuilder, SPIRV::Decoration::Offset, I,
                           {static_cast<uint32_t>(Offsets[I])});

@github-actions
Copy link

github-actions bot commented May 6, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@s-perron s-perron merged commit 097fef2 into llvm:main May 6, 2025
12 checks passed
@kstoimenov
Copy link
Contributor

@s-perron I reverted this and the other related change FYI...

s-perron added a commit to s-perron/llvm-project that referenced this pull request May 7, 2025
The asan failure was fixed by llvm#138695, but another failure was
introduced in the meantime. The cause for the other failure has been
fixed. I will reapply the two PRs.

Reapply "[SPIRV] Add explicit layout (llvm#135789)"

This reverts commit 0fb5720.

Reapply "[SPIRV] Fix asan failure (llvm#138695)"

This reverts commit df90ab9.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
When the DataLayout is destroyed, the memory backing `Offsets` is
released. This causes a use after free.

To fix it, I added a DataLayout varible that will not be destroyed until
after Offsets is used.

Fixes asan failure caused by
llvm#135789
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
s-perron added a commit that referenced this pull request May 7, 2025
The asan failure was fixed by #138695, but another failure was
introduced in the meantime. The cause for the other failure has been
fixed. I will reapply the two PRs.

Reapply "[SPIRV] Add explicit layout (#135789)"

This reverts commit 0fb5720.

Reapply "[SPIRV] Fix asan failure (#138695)"

This reverts commit df90ab9.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants